luci-app-lxc: code cleanup

Number of suggested rewrites:
* use lua ubus bindings instead of os.execute
* combine multiple actions to use same handler 'lxc_action' and pass
actions as arguments to consolidate code
* read openwrt version from lua directly
* start using String.prototype.format for string formatting

Also, drop 'rename' and 'clone' until they are fully functional on all
platforms.

Thanks Jo-Philipp for suggestions on how to improve the code.

Signed-off-by: Petar Koretic <[email protected]>

diff --git a/utils/luci-app-lxc/files/controller/lxc.lua b/utils/luci-app-lxc/files/controller/lxc.lua
index 8172fb9c0e4ac9cbca9d67bfe5ae330f839ee10e..ce7b53e6eadd4c30efb6ee3aeed82b1e9bcddb57 100644 (file)
--- a/utils/luci-app-lxc/files/controller/lxc.lua
+++ b/utils/luci-app-lxc/files/controller/lxc.lua
@@ -16,6 +16,37 @@ Author: Petar Koretic <[email protected]>
 
 module("luci.controller.lxc", package.seeall)
 
+require "ubus"
+local conn = ubus.connect()
+if not conn then
+    error("Failed to connect to ubus")
+end
+
+
+function fork_exec(command)
+       local pid = nixio.fork()
+       if pid > 0 then
+               return
+       elseif pid == 0 then
+               -- change to root dir
+               nixio.chdir("/")
+
+               -- patch stdin, out, err to /dev/null
+               local null = nixio.open("/dev/null", "w+")
+               if null then
+                       nixio.dup(null, nixio.stderr)
+                       nixio.dup(null, nixio.stdout)
+                       nixio.dup(null, nixio.stdin)
+                       if null:fileno() > 2 then
+                               null:close()
+                       end
+               end
+
+               -- replace with target command
+               nixio.exec("/bin/sh", "-c", command)
+       end
+end
+
 function index()
        page = node("admin", "services", "lxc")
        page.target = cbi("lxc")
@@ -25,31 +56,7 @@ function index()
        page = entry({"admin", "services", "lxc_create"}, call("lxc_create"), nil)
        page.leaf = true
 
-       page = entry({"admin", "services", "lxc_stop"}, call("lxc_stop"), nil)
-       page.leaf = true
-
-       page = entry({"admin", "services", "lxc_start"}, call("lxc_start"), nil)
-       page.leaf = true
-
-       page = entry({"admin", "services", "lxc_reboot"}, call("lxc_reboot"), nil)
-       page.leaf = true
-
-       page = entry({"admin", "services", "lxc_delete"}, call("lxc_delete"), nil)
-       page.leaf = true
-
-       page = entry({"admin", "services", "lxc_list"}, call("lxc_list"), nil)
-       page.leaf = true
-
-       page = entry({"admin", "services", "lxc_rename"}, call("lxc_rename"), nil)
-       page.leaf = true
-
-       page = entry({"admin", "services", "lxc_clone"}, call("lxc_clone"), nil)
-       page.leaf = true
-
-       page = entry({"admin", "services", "lxc_freeze"}, call("lxc_freeze"), nil)
-       page.leaf = true
-
-       page = entry({"admin", "services", "lxc_unfreeze"}, call("lxc_unfreeze"), nil)
+       page = entry({"admin", "services", "lxc_action"}, call("lxc_action"), nil)
        page.leaf = true
 
        page = entry({"admin", "services", "lxc_configuration_get"}, call("lxc_configuration_get"), nil)
@@ -67,87 +74,23 @@ function lxc_create(lxc_name, lxc_template)
 
        local url = uci:get("lxc", "lxc", "url")
 
-       local f = io.popen([[grep DISTRIB_TARGET /etc/openwrt_release | awk -F"[/'']" '{ print $2 }']])
-       if not f then
+       if not pcall(dofile, "/etc/openwrt_release") then
                return luci.http.write("1")
        end
 
-       local target = f:read("*all")
+       local target = _G.DISTRIB_TARGET:match('([^/]+)')
 
-       local res = os.execute("lxc-create -t download -n " .. lxc_name .. " -- --server=" .. url .. " --no-validate --dist openwrt --release bb --arch " .. target)
+       local res = os.execute("lxc-create -t download -n " .. lxc_name .. " -- --server=" .. url .. " --no-validate --dist " .. lxc_template .. " --release bb --arch " .. target)
 
        luci.http.write(tostring(res))
 end
 
-function lxc_start(lxc_name)
-       luci.http.prepare_content("text/plain")
-
-       local res = os.execute("ubus call lxc start '{\"name\" : \"" .. lxc_name .. "\"}' ")
-
-       luci.http.write(tostring(res))
-end
-
-function lxc_stop(lxc_name)
-       luci.http.prepare_content("text/plain")
-
-       local res = os.execute("ubus call lxc stop '{\"name\" : \"" .. lxc_name .. "\"}' ")
-
-       luci.http.write(tostring(res))
-end
-
-function lxc_delete(lxc_name)
-       luci.http.prepare_content("text/plain")
-
-       os.execute("ubus call lxc stop '{\"name\" : \"" .. lxc_name .. "\"}' ")
-       local res = os.execute("ubus call lxc destroy '{\"name\" : \"" .. lxc_name .. "\"}' ")
-
-       luci.http.write(tostring(res))
-end
-
-function lxc_reboot(lxc_name)
-       luci.http.prepare_content("text/plain")
-
-       local res = os.execute("ubus call lxc reboot '{\"name\" : \"" .. lxc_name .. "\"}' ")
-
-       luci.http.write(tostring(res))
-end
-
-function lxc_rename(lxc_name_cur, lxc_name_new)
-       luci.http.prepare_content("text/plain")
-
-       local res = os.execute("ubus call lxc rename '{\"name\" : \"" .. lxc_name_cur .. "\", \"newname\" : \"" .. lxc_name_new .. "\"}' ")
-
-       luci.http.write(tostring(res))
-end
-
-function lxc_freeze(lxc_name)
-       luci.http.prepare_content("text/plain")
-
-       local res = os.execute("ubus call lxc freeze '{\"name\" : \"" .. lxc_name .. "\"}' ")
-
-       luci.http.write(tostring(res))
-end
-
-function lxc_unfreeze(lxc_name)
-       luci.http.prepare_content("text/plain")
-
-       local res = os.execute("ubus call lxc unfreeze '{\"name\" : \"" .. lxc_name .. "\"}' ")
-
-       luci.http.write(tostring(res))
-end
-
-function lxc_list()
+function lxc_action(lxc_action, lxc_name)
        luci.http.prepare_content("application/json")
 
-       local cmd = io.popen("ubus call lxc list")
-       if not cmd then
-               return luci.http.write("{}")
-       end
-
-       local res = cmd:read("*all")
-       cmd:close()
+       local data, ec = conn:call("lxc", lxc_action, lxc_name and { name = lxc_name} or {} )
 
-       luci.http.write(res)
+       luci.http.write_json(ec and {} or data)
 end
 
 function lxc_configuration_get(lxc_name)

diff --git a/utils/luci-app-lxc/files/view/lxc.htm b/utils/luci-app-lxc/files/view/lxc.htm
index 1d38721da6223354fbd7997e20070665c7305414..1376968e3b7f4b6474c4f68c4a8a614753b490b9 100644 (file)
--- a/utils/luci-app-lxc/files/view/lxc.htm
+++ b/utils/luci-app-lxc/files/view/lxc.htm
@@ -98,7 +98,7 @@ Author: Petar Koretic <[email protected]>
 
                loading(loader_add)
 
-               new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_create/' + lxc_name + "/" + lxc_template , null,
+               new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_create/' + '%h/%h'.format(lxc_name, lxc_template) , null,
                function(x)
                {
                        bt_create.disabled = false
@@ -118,7 +118,7 @@ Author: Petar Koretic <[email protected]>
                var actions = ''
                actions += '<input type="button" onclick="action_handler(this)" data-action="start" value="<%:Start%>" class="cbi-button cbi-button-apply" />'
                actions+= '<input type="button" onclick="action_handler(this)" data-action="stop" value="<%:Stop%>" class="cbi-button cbi-button-reset" />'
-               actions+= '<input type="button" onclick="action_handler(this)" data-action="delete" value="<%:Delete%>" class="cbi-button cbi-button-remove" />'
+               actions+= '<input type="button" onclick="action_handler(this)" data-action="destroy" value="<%:Delete%>" class="cbi-button cbi-button-remove" />'
                actions+= ' <select class="cbi-input-select cbi-button" onchange="action_more_handler(this)">\
                                                <option selected disabled>more</option>\
                                                <option>configure</option>\
@@ -130,7 +130,7 @@ Author: Petar Koretic <[email protected]>
 
                var row = t_lxc_list.insertRow(-1)
                var cell = row.insertCell(-1)
-               cell.innerHTML = "<strong>" + lxc_name + "</strong>"
+               cell.innerHTML = '%q%h%q'.format("<strong>", lxc_name, "</strong>")
                cell.width = "30%"
                cell.setAttribute("data-id", lxc_name)
 
@@ -158,14 +158,14 @@ Author: Petar Koretic <[email protected]>
                {
                        loading(loader)
 
-                       new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_stop/' + lxc_name, null,
-                       function(x)
+                       new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/' + '%h/%h'.format(action, lxc_name), null,
+                       function(x, ec)
                        {
                                loading(loader, 0)
                                bt_action.disabled = false
 
-                               if (!x || x.responseText != "0")
-                                       return info_message(output_list,"Invalid response from system!")
+                               if (!x || ec)
+                                       return info_message(output_list,"Action failed!")
 
                                set_status(status_img, "red")
 
@@ -176,36 +176,36 @@ Author: Petar Koretic <[email protected]>
                {
                        loading(loader)
 
-                       new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_start/' + lxc_name, null,
-                       function(x)
+                       new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/' + '%h/%h'.format(action, lxc_name), null,
+                       function(x, data)
                        {
                                loading(loader, 0)
                                bt_action.disabled = false
 
                                //FIXME: uncomment after fixing 'lxc-start'
-                               if (!x /*|| x.responseText != "0"*/)
-                                       return info_message(output_list,"Invalid response from system!")
+                               if (!x /*|| ec */)
+                                       return info_message(output_list,"Action failed!")
 
                                //FIXME: uncomment after fixing 'lxc-start'
                                //set_status(status_img, "green")
                        });
                }
 
-               else if (action == "delete")
+               else if (action == "destroy")
                {
                        if (!confirm("This will completely remove LXC container from the disk. Are you sure? (container will be stopped if running)"))
                                return
 
                        loading(loader)
 
-                       new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_delete/' + lxc_name, null,
-                       function(x)
+                       new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/' + '%h/%h'.format(action, lxc_name), null,
+                       function(x, ec)
                        {
                                loading(loader, 0)
                                bt_action.disabled = false
 
-                               if (!x || x.responseText != "0")
-                                       return info_message(output_list,"Invalid response from system!")
+                               if (!x || ec)
+                                       return info_message(output_list,"Action failed!")
 
                                var row = self.parentNode.parentNode
                                row.parentNode.removeChild(row)
@@ -214,26 +214,6 @@ Author: Petar Koretic <[email protected]>
                }
        }
 
-       function lxc_rename_handler(self)
-       {
-               var td = self.parentNode
-               var lxc_name_holder = td.querySelector('[data-id]')
-               var lxc_name_cur = lxc_name_holder.getAttribute('data-id')
-               var lxc_name_new = lxc_name_holder.value
-
-               if (t_lxc_list.querySelector("[data-id='" + lxc_name_new + "']") != null)
-                       return info_message(output_list, "Container with new name already exists!", 4000)
-
-               new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_rename/' + lxc_name_cur + '/' + lxc_name_new, null,
-               function(x)
-               {
-                       if (!x || x.responseText != "0")
-                               return info_message(output_list,"Invalid response from system!")
-
-                       info_message(output_list,"LXC renamed")
-               })
-       }
-
        function lxc_configure_handler(self)
        {
                var td = self.parentNode
@@ -245,7 +225,7 @@ Author: Petar Koretic <[email protected]>
                function(x)
                {
                        if (!x || x.responseText != "0")
-                               return info_message(output_list,"Invalid response from system!")
+                               return info_message(output_list,"Action failed!")
 
                        info_message(output_list,"LXC configuration updated")
                        var row = td.parentNode
@@ -299,19 +279,6 @@ Author: Petar Koretic <[email protected]>
 
                        break
 
-                       case "rename":
-                               var tr = document.createElement('tr')
-                               var row = self.parentNode.parentNode
-                               var next_row = row.nextSibling
-                               if (next_row && next_row.getAttribute('data-action') !== null)
-                                       row.parentNode.removeChild(next_row)
-
-                               tr.innerHTML="<td colspan='" + row.cells.length + "'>" + lxc_rename_template(lxc_name) + "</td>"
-                               tr.setAttribute('data-action','')
-                               row.parentNode.insertBefore(tr, row.nextSibling)
-
-                               break
-
                        case "freeze":
                                var tr = self.parentNode.parentNode
                                var img = tr.querySelector('img')
@@ -319,12 +286,12 @@ Author: Petar Koretic <[email protected]>
                                        return info_message(output_list,"Container is not running!")
 
                                loading(loader)
-                               new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_freeze/' + lxc_name, null,
-                               function(x)
+                               new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/' + '%h/%h'.format(option, lxc_name), null,
+                               function(x, ec)
                                {
                                        loading(loader, 0)
-                                       if (!x || x.responseText != "0")
-                                               return info_message(output_list,"Invalid response from system!")
+                                       if (!x || ec)
+                                               return info_message(output_list,"Action failed!")
 
                                        set_status(img, "purple")
                                })
@@ -339,12 +306,12 @@ Author: Petar Koretic <[email protected]>
                                        return info_message(output_list,"Container is not frozen!")
 
                                loading(loader)
-                               new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_unfreeze/' + lxc_name, null,
-                               function(x)
+                               new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/' + '%h/%h'.format(option, lxc_name), null,
+                               function(x, ec)
                                {
                                        loading(loader, 0)
-                                       if (!x || x.responseText != "0")
-                                               return info_message(output_list,"Invalid response from system!")
+                                       if (!x || ec)
+                                               return info_message(output_list,"Action failed!")
 
                                        set_status(img, "green")
                                })
@@ -361,12 +328,12 @@ Author: Petar Koretic <[email protected]>
                                        return
 
                                loading(loader)
-                               new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_reboot/' + lxc_name, null,
-                               function(x)
+                               new XHR().get('<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/' + '%h/%h'.format(option, lxc_name), null,
+                               function(x, ec)
                                {
                                        loading(loader, 0)
-                                       if (!x || x.responseText != "0")
-                                               return info_message(output_list,"Invalid response from system!")
+                                       if (!x || ec)
+                                               return info_message(output_list,"Action failed!")
 
                                        info_message(output_list,"LXC rebooted")
                                })
@@ -392,7 +359,7 @@ Author: Petar Koretic <[email protected]>
 
        function lxc_list_update()
        {
-               XHR.poll(4, '<%=luci.dispatcher.build_url("admin", "services")%>/lxc_list', null,
+               XHR.poll(4, '<%=luci.dispatcher.build_url("admin", "services")%>/lxc_action/list', null,
                function(x, data)
                {
                        if (!x) return;